-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix util.promisify(setTimeout) on jest-fake-timers #9180
Conversation
Hi lourenci! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Codecov Report
@@ Coverage Diff @@
## master #9180 +/- ##
==========================================
- Coverage 65.06% 64.83% -0.24%
==========================================
Files 278 279 +1
Lines 11864 11743 -121
Branches 2924 2884 -40
==========================================
- Hits 7719 7613 -106
+ Misses 3518 3511 -7
+ Partials 627 619 -8
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this! I left comments where the code/test does not seem to be correct
Can you add a changelog entry please? :) |
Made an edit that triggers Azure CI to run again, I think the macOS cancellation was just flakiness, let's see |
Alright, Azure passed. Node 13 failure happens everywhere now, unrelated. Thanks a lot for the contribution! :) |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
It closes #9150.
Some points to consider and help me to reach the best implementation.
setTimeout[util.promisify.custom]
onjest-fake-timers
like the original one implemented on node. It doesn't call the callback passed through it. Therefore this implementation differs from Lolex implementation.runTimers...
. I still think this is the best strategy because it looks like it is more "black-boxed", but I did the implementation with the least effort to get some advice.